fix(patterns): mask dates before Drain by bcanvural · Pull Request #3551 · ClickHouse/ClickStack

bcanvural · 2026-11-04T16:03:32Z

Refs #3544 (option 1)

Option 2 (comparing both windows in one Drain tree) moved to #3556 / Draft #3557, stacked on this PR. Option 3 (bodyExpression) is left for a new issue after #3426, as agreed on #3544. Merged with main after #3426 landed: the drill-down uses #3426's predicate.

What

Default masking before Drain (packages/common-utils/src/drain/config.ts). TemplateMinerConfig starts with DEFAULT_MASKING_INSTRUCTIONS, which mask only the date of a timestamp to <DATE> (exported as DATE_MASK): ISO dates (also before T in ISO-8601), RFC-1123, ctime and Java Date.toString, JavaScript Date.toString, java.util.logging, syslog, access log, Tomcat, dd/MM/yyyy, MM/dd/yyyy, dd.MM.yyyy, Spark yy/MM/dd and the klog I1104 date. Forms other than ISO dates are masked only before a time.

Times and ids are not masked. Every line of a day shares its date, and <DATE> matches itself the same way, so within one day lines group as without masks (tested); across days the templates are the same. One change within a day: klog lines with the same message but different severity letters are no longer merged, because I<DATE> and E<DATE> route by their letter.

No mask changes a line's token count: month-name dates are masked word by word through a new optional perWord flag on masking instructions (default off, as in drain3). Every caller that builds a TemplateMinerConfig (pattern mining, denoise, the CLI) gets the masks; TemplateMinerConfig.fromJSON applies none unless the JSON lists them, as drain3 does.
Drill-down (templateToRegex in patternDrilldown.ts): <DATE> matches one or more non-whitespace characters, also inside a token (backup_<DATE>.tar). A whole-token <*> is unchanged, so fix(mcp): make event_patterns drill-down snippets executable #3426's matching rules and tests stay.
Denoise (buildNoisyBodyMatcher): masks each body the same way before comparing it with the noisy templates.

The web app's Event patterns view runs drain3 in Pyodide with its own config and is unchanged.

Why

On dated logs (Java/Logback, Python, Airflow run ids), emerging_signals with a yesterday baseline reported the same pattern as both emerging and disappeared, because the templates differed only by the date (#3544: 19–135 false patterns per run on 4 services).

Tests

drain-masking.test.ts: each mask on real-looking lines; lines left alone (versions, hex, short shas, times, ids, 5 Mar, 10.11.2026 without a time, 1234-56-78); a mixed day of logs clusters the same with and without masks (klog has its own test); two days and a month boundary give the same templates; word counts never change; extractParameters reads a per-word date back; drain3 fromJSON behaviour; linear time on a 128 KiB whitespace run.
patternDrilldown.test.ts: <DATE> as a word and inside a token, mined dated lines whose predicate matches every own sample, and a dated line next to Started job at boot.
denoise.test.ts: dated noisy lines are removed and a rare line is kept.
3,090 common-utils tests, 560 MCP unit tests and the emerging_signals and query tool integration tests (116) pass; tsc, eslint and prettier clean.

…gnals windows in one tree Dated log lines kept their literal date in the template, and emerging_signals mined the baseline and current windows in separate trees and matched them by template text, so the same line showed up as both new and gone. Mask timestamps (ISO-8601/RFC-3339, RFC-1123, access-log, syslog), UUIDs and long hex ids by default, mine both windows with one TemplateMiner and compare by cluster id. End the baseline 1 ms early so a row at the shared boundary isn't counted in both windows. Refs ClickHouse#3544.

changeset-bot · 2026-11-04T16:03:37Z

🦋 Changeset detected

Latest commit: 3eb9ffc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@hyperdx/common-utils	Patch
@hyperdx/api	Patch
@hyperdx/cli	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2026-11-04T16:03:39Z

@bcanvural is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2026-11-04T16:06:32Z

+        miner: new TemplateMiner(new TemplateMinerConfig()),
      };

      const [curRes, baseRes] = await Promise.all([


🟠 major — The shared Drain tree is filled in whichever order the two ClickHouse queries finish, so emerging_signals output can differ between runs

mineWindowPatterns mines synchronously right after its own queries resolve (runEventPatterns.ts:237). With Promise.all, whichever window's queries return first goes into the shared miner first. Drain depends on insertion order: clusters, ids and templates come out differently, so identical inputs can give different emerging/disappeared lists. Also, when the current window is mined first, its pattern (taken first in ingest) is the cluster's template before the baseline rows generalized it. Fix: fetch both windows in parallel, then mine in a fixed order (baseline first, then current). One way is to split fetching from mining in mineWindowPatterns; the simpler way is to run the two calls one after the other. Then read each cluster's template from the miner once both windows are in.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

github-actions · 2026-11-04T16:06:32Z

          input.sourceId,
          base.startDate,
-          base.endDate,
+          new Date(base.endDate.getTime() - 1),


🔵 minor — Ending the baseline 1 ms early silently drops rows with sub-millisecond timestamps in that last millisecond

Source timestamps are usually DateTime64(9). Rows in (baselineEnd − 1 ms, baselineEnd) now fall outside the baseline (<= end−1ms) and outside the current window (>= currentStart). The summary still reports base.endDate as the end. Fix: keep base.endDate and pass an exclusive end through to the window query instead. The chart config already supports this as dateRangeEndInclusive: false (packages/common-utils/src/types.ts:2056, honored in renderChartConfig.ts:1062), so add a dateRangeEndInclusive option to mineWindowPatterns and set it on the baseline sample and count configs.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-11-04T16:06:32Z

  maskPrefix: string = '<';
  maskSuffix: string = '>';
-  maskingInstructions: MaskingInstructionConfig[] = [];
+  maskingInstructions: MaskingInstructionConfig[] = [


🔵 minor — The web app's Event patterns and denoise don't get the new masks, so the changeset's 'Event patterns no longer split' claim is wrong there

The UI's Event patterns and the DBRowTable denoise run Python drain3 through Pyodide with their own config (packages/app/src/hooks/usePatterns.tsx:62-63), so DEFAULT_MASKING_INSTRUCTIONS never reaches them. Dated lines still split in the UI, and web denoise now groups rows differently from MCP denoise, even though mine-patterns.ts says the denoise constants are shared between the two paths. Fix: either pass the same masking instructions into the Pyodide TemplateMinerConfig, or limit the changeset wording to the MCP/CLI paths.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-11-04T16:06:34Z

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.

🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.

Anything about surrounding code, or asking you to widen the change (hoist a helper,
dedupe with another file, fix other call sites): never fix here, whatever the
severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

1 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 1 minor

1 posted as inline comment(s) on the changed lines.

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

github-actions · 2026-11-04T16:13:16Z

Deep Review

✅ No critical issues found.

This is a well-scoped, well-tested change. The default date-masking instructions, the perWord flag, and the <DATE>-aware drill-down regex and denoise matcher are internally consistent: mining masks via LogMasker.fromConfig(new TemplateMinerConfig()) and the denoise matcher masks the flattened body the same way via createPatternMasker(), so a mined template and a raw body are compared after identical masking. Token-count preservation (perWord), instruction ordering (ctime-year before month/day), negative cases (v1.2.3, 0x1f, time-only 10:30, UUIDs), in-token masks (backup_<DATE>.tar), and ReDoS linearity are all covered by tests.

🟡 P2 -- recommended

packages/common-utils/src/drain/config.ts:63 -- Ambiguous numeric date formats (dd/MM/yyyy, MM/dd/yyyy, dd.MM.yyyy, yy/MM/dd) are masked only when immediately followed by a time token, so a dated log in those formats without an adjacent time still fragments into one pattern per day and is not deduplicated by emerging_signals.
- Fix: Document this intentional conservatism in the instruction comment and confirm the targeted log sources in emerging_signals reports dated log lines as both "new" and "gone": Drain keeps the literal date, and each window is mined separately #3544 all emit a time adjacent to the date; otherwise consider a follow-up for time-less numeric dates.

🔵 P3 nitpicks (2)

packages/common-utils/src/drain/config.ts:85 -- maskingInstructions = [...DEFAULT_MASKING_INSTRUCTIONS] creates a per-instance array but its elements are the same shared MaskingInstructionConfig object references; mutating an element (rather than reassigning the array) would leak across every TemplateMinerConfig instance and into the exported default.
- Fix: Freeze DEFAULT_MASKING_INSTRUCTIONS (and its elements) or deep-copy them in the field initializer so element mutation cannot alias the shared default.
packages/api/src/mcp/tools/query/patternDrilldown.ts:9 -- DATE_TOKEN in the drill-down is reconstructed from the imported DATE_MASK while mining builds the token from maskPrefix/maskSuffix; the coupling that keeps these equal (< / >) is implicit, so a future change to the mask delimiters would silently desync the predicate from mined templates.
- Fix: Derive the drill-down token from the same TemplateMinerConfig prefix/suffix (or add a test asserting the mined <DATE> token equals DATE_TOKEN).

Reviewers (7): correctness, adversarial, testing, maintainability, performance, kieran-typescript, previous-comments.

Testing gaps:

No test asserts behavior when a log body literally contains the substring <DATE> or <*>; both mining and the drill-down would treat it as a mask placeholder (very low likelihood, shared with the pre-existing <*> handling).
Prior-comment threads (masks inside a token, backup_<DATE> lookaround fix, fromJSON masks-cleared) appear addressed in the current diff and are covered by the new tests.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

…pen baseline, drop masked tokens from whereSnippet Fetch both windows in parallel, then mine baseline and current into the shared miner in that order, so the result doesn't depend on which query finishes first; report each cluster's template from the finished tree. End the baseline with an exclusive bound instead of 1 ms early. Drop tokens that contain a mask from whereSnippet. Narrow the changeset to the MCP/CLI paths (the web app's Event patterns view uses drain3 with its own config).

bcanvural · 2026-11-04T16:38:34Z

Thanks for the reviews. Addressed in 8c44344:

🟠 whereSnippet with masks inside a token ([<*>], run_id=scheduled__<*>, trace_id=<*>): the builder now drops any token that contains <*>, not only bare <*> tokens (extracted as buildWhereSnippet, with a test for a bracketed timestamp and key=id tokens). Kept minimal, since fix(mcp): make event_patterns drill-down snippets executable #3426 reworks this builder.
🟠 Insertion order in the shared tree: the windows are now fetched in parallel and then mined in a fixed order, baseline first (mineWindowPatterns is split into fetchWindowSample + mineWindowSample), and each cluster's reported template is read from the finished tree. New unit test: same result whichever window query finishes first.
🔵 / P2 1 ms early baseline end: replaced with a half-open baseline (dateRangeEndInclusive: false), so no sub-millisecond rows are dropped and the reported end is the queried end.
🔵 Web app Event patterns: it runs drain3 in Pyodide with its own config, so rather than widen this PR I narrowed the changeset to the MCP and CLI paths.
P2 product-wide default: intended, so MCP event_patterns, MCP denoise and the CLI mine and match with the same masks; the default is covered by drain-masking.test.ts ("is on by default" plus the minePatterns cases). Happy to make it opt-in per caller if you'd prefer.

Tests: 489 api MCP unit tests, emergingSignalsTool.int.test.ts 5/5 and the pattern/denoise cases in queryTool.int.test.ts 13/13 pass; tsc, eslint and prettier clean.

github-actions · 2026-11-04T16:41:59Z

+ * pattern → search. Tokens that contain a mask (`<*>`, also inside a token such
+ * as `[<*>]` or `trace_id=<*>`) are dropped, since their text varies per line.
+ * Escapes \ and " (the phrase-query metachars) in each token.
 */


🔵 minor — whereSnippet joins non-adjacent tokens into a field phrase, which compiles to an exact match on Body

A quoted Body:"..." term goes through serializer.eq (packages/common-utils/src/queryParser.ts:2076), so ClickHouse gets Body = 'task finished in ms'. That never matches a line whose masked tokens were dropped from between the words. The masks make this the common case: most dated lines now produce a masked token. Have the snippet emit one term per literal token, e.g. Body:*task* Body:*finished*, or use implicit-field substring terms. Then change the new whereSnippet.test.ts so it stops pinning the output that cannot match.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-11-04T16:41:59Z

      ingest(baseRes, 'base');
+      // A cluster's template can still generalize after a window first sees
+      // it, so report each cluster's template from the finished tree.
+      for (const a of byKey.values()) {


🔵 minor — Reported template comes from re-matching the sample, not from the cluster the row is keyed by

byKey is keyed by cluster id, but miner.match(sample, 'fallback') searches the tree again. That search can land on a different cluster: a literal child added later can divert the tree walk, and the fastMatch tie-break prefers the candidate with more params. Look the template up by id instead. One way: expose a getCluster(id) on Drain (it already has a private clusterPeek) and read getCluster(Number(key)).getTemplate().

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

…, UUID and hex masks \b doesn't match between _ and a digit, so backup_2026-11-04 and span_<hex> kept their dates and ids. Use alphanumeric lookarounds instead.

bcanvural · 2026-11-04T16:45:47Z

Round 2:

Fixed: the bare date, time, UUID and hex masks used \b, which doesn't match between _ and a digit, so backup_2026-11-04 and span_<hex> kept their dates and ids. They now use alphanumeric lookarounds; tests added for both (and a req_<uuid> case).
Not changed, to keep the scope: splitting WindowOptions into fetch/mine types, and reading templates through a Drain.getCluster instead of match(). Happy to do either if you prefer.
Body:"…" snippets compiling to exact = predates this PR and is what fix(mcp): make event_patterns drill-down snippets executable #3426 reworks.

That's two automated rounds, so per AGENTS.md I'll stop here. @brandon-pereira, ready for a look when you have time.

github-actions · 2026-11-04T16:48:05Z

+      // Mine in a fixed order (baseline, then current): Drain depends on
+      // insertion order, so this keeps the result independent of which query
+      // finished first.
+      const miner = new TemplateMiner(new TemplateMinerConfig());


🟠 major — With one Drain tree, a new message that resembles a baseline message no longer counts as emerging

Drain merges any same-length line with ≥40% token overlap (drainSimTh 0.4) into the existing cluster. Example: the baseline has user login failed: invalid password and the current window adds user login failed: account locked. Both now land in the baseline's cluster (user login failed: <*> <*>), so the new error shows up as at most 'shifted' and usually not at all. Before this change, separate trees gave them different templates and the new one was reported as 'new'. Fix: detect generalization caused by the current window. For example, record addLogMessage().changeType === 'cluster_template_changed' for current-window rows on clusters first created by the baseline, or also compare the baseline-only template against the final one, and surface those clusters. Add a test with same-length, mostly overlapping messages; the existing new/gone tests only use lines with different token counts.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

github-actions · 2026-11-04T16:48:06Z

+      // A cluster's template can still generalize after a window first sees
+      // it, so report each cluster's template from the finished tree.
+      for (const a of byKey.values()) {
+        a.pattern =


🔵 minor — Re-reading the template with miner.match(sample) can return a different cluster than a's id

Drain.match (packages/common-utils/src/drain/drain.ts:332) returns the 1.0-similarity cluster with the most params, and with 'fallback' it searches every cluster of that length. So a more general cluster can win, and two different Aggs can report the same pattern string. The cluster id (byKey key) is already known, so look the cluster up by id instead: expose a public getCluster(id) on Drain/TemplateMiner that wraps clusterPeek, and use getTemplate() on the result.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-11-04T16:48:06Z

+    expect(all).toEqual(
+      expect.arrayContaining([
+        expect.stringContaining('disk almost full'),
+        expect.stringContaining('cache warmed'),


🔵 minor — Order-independence test would still pass if mining followed fetch completion order

The baseline and current fixtures have no overlapping clusters that generalize differently depending on which window goes first, and final templates are re-read from the finished tree. So mining each window inside its fetch's .then (completion order) would give an identical result. Use fixtures where insertion order changes the outcome, e.g. one window holds the constant host=db-1 lines and the other the varying ones, with the varying set first. Assert that the output matches the baseline-first result.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

…n as new_variant With one Drain tree, a current-window message similar to a baseline one (same length, enough shared tokens) is merged into the baseline cluster and only generalizes its template, so it was no longer reported as emerging. Snapshot each baseline template before mining the current window; where the current window changed it, the current rows that fit the new template but not the old one are reported as status new_variant, with the old template and a sample. Read each cluster's final template by id (Drain.getCluster) instead of re-matching the sample.

bcanvural · 2026-11-04T17:03:59Z

One more round, out of the usual two, because the latest review found a regression this PR introduced: with one Drain tree, a new message that resembles a baseline one (e.g. user login failed: account locked next to user login failed: invalid password) was merged into the baseline pattern and stopped being reported. Fixed in 80ce437:

Each baseline cluster's template is recorded before the current window is mined. Where the current window changed it, the current rows that fit the new template but not the old one are reported in emerging with status: "new_variant", the old template as baselinePattern, a new line as sample, and their share as variantShare. Masked dates and ids don't change templates, so dated lines still don't show up.
Final templates are read by cluster id (Drain.getCluster) instead of re-matching the sample (the two minors on this).
Tests (emergingSignalsOrder.test.ts): the login example is reported as new_variant; the same dated line on two days gives nothing; identical windows give nothing; an account that was constant in the baseline and different now gives one new_variant entry instead of a "new" + "gone" pair (a real change). The order test now uses that case, so it would fail if mining followed query completion order. 492 MCP unit tests, the emerging_signals int tests (5/5) and the pattern/denoise int cases (13/13) pass.

Replied rather than changed: the Body:"…" whereSnippet form (pre-existing, reworked by #3426), and the web app's Pyodide drain3 config (out of scope; the changeset says the UI view is unchanged).

I'll stop here. @brandon-pereira, ready for a look when you have time.

github-actions · 2026-11-04T17:08:07Z

-        status:
-          a.baseShare === 0 ? 'new' : a.curShare === 0 ? 'gone' : 'shifted',
-        sample: a.sample.slice(0, 300),
+        status: a.variant


🔵 minor — Entries in disappeared can carry status: "new_variant"

a.variant is set whenever any current row fits the widened template. When variant.share is below newPatternShareFloor, the verdict falls back to classifyShift, which can return disappeared (the baseline share dropped ≥ minShareRatio×). fmt still labels the entry new_variant, with baselinePattern/variantShare, inside the disappeared list, which contradicts the usage text. Fix: set new_variant (and the extra fields) only when the variant itself drove the emerging verdict. For example, clear a.variant when share < newPatternShareFloor, or base the status on the verdict.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

…oves to its own PR Review showed that comparing both windows in one Drain tree needs new-variant detection of its own, so it moves to a follow-up. This PR keeps the default masks (now also ctime/Java, log4j, Spark, klog and Go dates; drain3 JSON configs without masks get none), the whereSnippet fix for tokens that contain a mask, and their tests. emerging_signals goes back to two separate trees.

bcanvural · 2026-11-04T18:12:41Z

I've narrowed this PR to masking (option 1 of #3544): the default masks, the whereSnippet fix for tokens that contain a mask, and their tests. The latest review showed that comparing both windows in one Drain tree needs new-variant detection of its own (a new message similar to a baseline one gets merged into it), which made this PR much bigger, so that part moved to #3556, with Draft PR #3557 stacked on this one. emerging_signals here keeps the two separate trees, as on main.

Diff against main is now 5 files, +225/−12. Tests: 3,065 common-utils, 486 MCP unit, and the emerging_signals + pattern/denoise integration tests pass.

bcanvural · 2026-11-04T18:19:44Z

On the remaining 🟠 (the whereSnippet phrase still compiles to an exact Body:"…" match): that exact-match behaviour predates this PR, and it's what #3426 reworks. This PR only stops in-token masks like trace_id=<*> from being copied into the snippet. If you'd rather #3426 handle snippets entirely, I'm happy to drop that part from here.

The deep-review P2s (a fromJSON pass-through test, the backslash-escape test, 16+ digit decimals matching the hex mask) are small; I'll leave them for your call rather than start another round.

Take ClickHouse#3426's drill-down predicate in runEventPatterns.ts; drop this PR's buildWhereSnippet and its test, which ClickHouse#3426 supersedes.

…n the drill-down regex ClickHouse#3426's predicate matches a whole-token <*> as exactly one word and escapes anything else literally, so a masked timestamp that spans spaces (2026-11-04 06:12:12,345) or a mask inside a token ([<*>], trace_id=<*>) matched nothing. Named masks keep Drain's clustering as it is (a mask is one token that is equal across lines), and the regex maps <TS> to any text and <ID> to one word, also inside a token. Whole-token <*> is unchanged.

bcanvural · 2026-11-05T07:40:56Z

Merged main now that #3426 has landed (fast-forward, no force-push):

The conflict was the drill-down snippet. I took fix(mcp): make event_patterns drill-down snippets executable #3426's predicate and dropped this PR's buildWhereSnippet and its test.
fix(mcp): make event_patterns drill-down snippets executable #3426's templateToRegex treats a whole-token <*> as one word and escapes everything else, so masked patterns matched nothing: a masked timestamp can span spaces (2026-11-04 06:12:12,345), and a mask can sit inside a token ([<*>], trace_id=<*>). So the default masks are now named, <TS> for timestamps and <ID> for UUIDs/hex ids, and the regex maps <TS> to any text and <ID> to one word (also inside a token). A whole-token <*> is unchanged.
Named rather than <*> because Drain counts <*> positions against similarity: token-per-word <*> masks split lines with long dates (RFC-1123, ctime) into one pattern per value.

New tests mine dated Java, access-log, trace_id= and syslog lines with the default masks and check that each predicate matches its own samples and no other pattern's; they fail on main's regex. The same predicates matched exactly their own samples in ClickHouse.

github-actions · 2026-11-05T07:43:59Z

+            ? escapeRe2Literal(piece)
+            : piece === TIMESTAMP_TOKEN
+              ? '.+?'
+              : `${NON_WS}+`,


🟠 major — A <*> that Drain merged over a masked timestamp only matches one word, so the drill-down misses the cluster's own samples

Masking runs before Drain, so Drain can merge <TS> with a different token in the same position. Example: Started job at 2026-11-04 06:12:12 (masked to Started job at <TS>) and Started job at boot share a leaf, and the template becomes Started job at <*>. The whole-token <*> maps to NON_WS+, so the raw sample (5 words) fails the predicate. That breaks the 'every sampled member matches' promise in PATTERN_MATCH_SEMANTICS, which held before because unmasked dates were separate tokens. Fix: either stop Drain from generalizing a mask token into <*>, or let a whole-token <*> also match the timestamp shapes (e.g. (?:NON_WS+|.+?)). Add a test that mines a <TS> line together with a non-timestamp line.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

github-actions · 2026-11-05T07:43:59Z

@@ -0,0 +1,12 @@
+---
+'@hyperdx/common-utils': patch
+'@hyperdx/api': patch


🔵 minor — Changeset does not bump @hyperdx/cli even though the CLI's behaviour changes

packages/cli bundles @hyperdx/common-utils (it is a devDependency; see packages/cli/package.json and TemplateMinerConfig at packages/cli/src/cli.tsx:2302, plus minePatterns in usePatternData.ts). The changeset text says 'and the CLI', but only common-utils and api are released. Add '@hyperdx/cli': patch so the CLI change ships.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Mask the date parts of a timestamp to <DATE> and leave times and ids to Drain, so a masked token compares like the date it replaced: equal within a day, equal across days. Fixes the drill-down for a <*> merged over a masked timestamp, and denoise now masks bodies like the miner does.

github-actions · 2026-11-05T11:43:38Z

          for (const mi of instructions) {
-            allowedPatterns.push(mi.pattern);
+            // A per-word mask stands for one word of its pattern's match.
+            allowedPatterns.push(mi.perWord ? '\\S+' : mi.pattern);


🔵 minor — Adding \S+ for per-word masks turns off exact matching for every <DATE> mask, not only the per-word ones

All seven default instructions use the same mask name DATE, so allowedPatterns becomes iso|\S+|year|.... Because \S+ is one of the alternatives, every <DATE> placeholder accepts any word. For example, extractParameters('wrote backup_<DATE>.tar', 'wrote backup_hello.tar') with exactMatching=true now returns ['hello'] when it should return null. Fix: give the per-word instructions their own mask name, or build a per-word alternative that only matches one word of that instruction's own pattern (e.g. a month or weekday name, a day, or a year) rather than a bare \S+. Nothing in production calls extractParameters today; only tests use it.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Leaving this as is: nothing in production calls extractParameters, and matching each per-word slot exactly would need a second pattern per instruction. \S+ still keeps each <DATE> to one word.

bcanvural · 2026-11-05T12:06:32Z

The latest commit masks only dates: the date parts of a timestamp become <DATE>. Every line of a day shares its date and <DATE> matches itself the same way, so within one day lines group as without masks (tested), and across days the templates are the same. <TS>/<ID> are gone.

Why not times and ids: they already differ from line to line, so Drain turns them into <*> as soon as a pattern has two lines, and they don't cause the split. Dates are what does: every line of a day shares one, so Drain keeps it as a literal, and it changes the next day. Masking ids anyway costs more than it fixes:

As a named mask (<ID>), every id compares as equal, so Drain merges lines it keeps apart today, and a <*> it puts over <ID> broke the drill-down (the 🟠 below).
As <*>, Drain counts the token as a difference, so lines that are mostly ids or timestamps (e.g. <trace id> ok) never merge. Each forms its own pattern with the same text, e.g. <*> <*> ok. emerging_signals then adds those patterns together, and denoise can't tell them apart.

What's left: a line seen only once keeps its id or time in its template, so it can still show as new and gone.

🟠 Drill-down: a mask now stays inside one word, so a <*> that Drain puts over <DATE> stands for one word, as templateToRegex expects. <DATE> matches one or more non-whitespace characters, also inside a token (backup_<DATE>.tar). Test: a dated line next to Started job at boot; each predicate matches all its samples.
🟠 Denoise: buildNoisyBodyMatcher masks each body with the masker minePatterns uses (createPatternMasker) before comparing. Test with dated noisy lines.
🔵 Changeset: adds @hyperdx/cli, and says the web app's Event patterns (drain3 in Pyodide, its own config) is unchanged.
P2 (16+ digit decimals matching the hex mask): the id masks are removed.
Testing gap: added the fromJSON round-trip test.
P3s: the semantics string uses the <DATE> constant, the regex now splits on the plain token string, and the word boundaries are named constants. Not done: a note on log lines that literally contain <DATE>; they fall under the documented superset.

One change within a day: klog lines with the same message but different severity letters are no longer merged, because I<DATE> and E<DATE> route by their letter.

On the review of this commit:

P2 (dd/MM/yyyy, MM/dd/yyyy, dd.MM.yyyy, yy/MM/dd only before a time): on purpose. Without a time they are easy to confuse with versions and other numbers (release 10.11.2026); ISO dates are masked anywhere. The emerging_signals reports dated log lines as both "new" and "gone": Drain keeps the literal date, and each window is mined separately #3544 sources (Logback, Python logging, Airflow) write ISO dates.
P3 (shared instruction objects): nothing mutates them, so not changed.
P3 (DATE_TOKEN delimiters): the drill-down tests mine dated lines with the default config and match each predicate against its samples, so a delimiter change fails them.

@brandon-pereira, ready for a look when you have time.

bcanvural · 2026-11-06T12:34:29Z

Moving this to draft to stay within the open-PR limit and make room for the fix for #3580 (PromQL dashboard variables fail on ClickHouse 26.10), which is more urgent for us. I'll mark this ready again when a slot frees.

bcanvural requested a review from a team as a code owner November 4, 2026 16:03

bcanvural requested a review from pulpdrew November 4, 2026 16:03

github-actions Bot added the external label Nov 4, 2026

github-actions Bot reviewed Nov 4, 2026

View reviewed changes

bcanvural mentioned this pull request Nov 4, 2026

clickstack_table: no limit, having or default order, so large grouped results come back trimmed in arbitrary order #3555

Closed

github-actions Bot reviewed Nov 4, 2026

View reviewed changes

fix(patterns): treat underscores as separators in the bare date, time…

249470d

…, UUID and hex masks \b doesn't match between _ and a digit, so backup_2026-11-04 and span_<hex> kept their dates and ids. Use alphanumeric lookarounds instead.

github-actions Bot reviewed Nov 4, 2026

View reviewed changes

This was referenced Nov 4, 2026

emerging_signals: compare both windows in one Drain tree, and report new variants of baseline patterns #3556

Open

feat(mcp): emerging_signals compares both windows in one Drain tree and reports new variants #3557

Draft

bcanvural mentioned this pull request Nov 4, 2026

emerging_signals reports dated log lines as both "new" and "gone": Drain keeps the literal date, and each window is mined separately #3544

Open

bcanvural changed the title ~~fix(patterns): mask timestamps and ids before Drain, mine emerging_signals windows in one tree~~ fix(patterns): mask timestamps and ids before Drain Nov 4, 2026

bcanvural added 2 commits November 5, 2026 10:32

Merge branch 'main' into fix/patterns-mask-dated-lines

60f5f65

Take ClickHouse#3426's drill-down predicate in runEventPatterns.ts; drop this PR's buildWhereSnippet and its test, which ClickHouse#3426 supersedes.

github-actions Bot reviewed Nov 5, 2026

View reviewed changes

bcanvural changed the title ~~fix(patterns): mask timestamps and ids before Drain~~ fix(patterns): mask dates before Drain Nov 5, 2026

github-actions Bot reviewed Nov 5, 2026

View reviewed changes

bcanvural marked this pull request as draft November 6, 2026 12:34

Conversation

bcanvural commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Tests

Uh oh!

changeset-bot Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Nov 4, 2026

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Uh oh!

github-actions Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deep Review

🟡 P2 -- recommended

Uh oh!

bcanvural commented Nov 4, 2026

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural commented Nov 4, 2026

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural commented Nov 4, 2026

Uh oh!

github-actions Bot Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural commented Nov 4, 2026

Uh oh!

bcanvural commented Nov 4, 2026

Uh oh!

bcanvural commented Nov 5, 2026

Uh oh!

github-actions Bot Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural commented Nov 5, 2026

Uh oh!

bcanvural commented Nov 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

bcanvural commented Nov 4, 2026 •

edited

Loading

changeset-bot Bot commented Nov 4, 2026 •

edited

Loading

github-actions Bot commented Nov 4, 2026 •

edited

Loading

github-actions Bot commented Nov 4, 2026 •

edited

Loading